Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add check for shared volumes in getVolumeStats #2419

Merged
merged 2 commits into from
Feb 21, 2024

Conversation

dahuang-purestorage
Copy link
Contributor

@dahuang-purestorage dahuang-purestorage commented Feb 20, 2024

What this PR does / why we need it:
Adds check for shared v4 volumes that have a different attach path than the assumed CSI volume path.

Which issue(s) this PR fixes (optional)
PWX-35351

Testing Notes
With the fix, we are able to see Used Space on OCP:
image

Copy link
Contributor

@adityadani adityadani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm after comment is addressed

@@ -365,7 +365,8 @@ func (s *OsdCsiServer) NodeGetVolumeStats(ctx context.Context, req *csi.NodeGetV

var attachPathMatch bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, I don't understand the reason for this path match check. Regardless of the attachPath we can always return the volume usage for every volume. The attach path does not dictate the volume usage.

@dahuang-purestorage dahuang-purestorage force-pushed the PWX-35351-fix-nodegetvolume-shared-vol branch from b4ed691 to b015850 Compare February 21, 2024 01:06
@dahuang-purestorage
Copy link
Contributor Author

chatted with @adityadani offline. Removing the attach_path check will break how CSI sanity, so let's keep the attach_path validation.

@dahuang-purestorage dahuang-purestorage merged commit 1ec45ab into master Feb 21, 2024
2 checks passed
dahuang-purestorage added a commit that referenced this pull request Feb 21, 2024
* add check for shared volumes in getVolumeStats

Adds check for shared v4 volumes that have a different attach path than the assumed CSI volume path.

JIRA: PWX-35351

Signed-off-by: dahuang <[email protected]>
dahuang-purestorage added a commit that referenced this pull request Feb 21, 2024
dahuang-purestorage added a commit that referenced this pull request Feb 21, 2024
* add check for shared volumes in getVolumeStats (#2419)

Adds check for shared v4 volumes that have a different attach path than the assumed CSI volume path.

JIRA: PWX-35351

Signed-off-by: dahuang <[email protected]>
cnbu-jenkins pushed a commit that referenced this pull request Feb 27, 2024
* add check for shared volumes in getVolumeStats

Adds check for shared v4 volumes that have a different attach path than the assumed CSI volume path.

JIRA: PWX-35351

Signed-off-by: dahuang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants